plan/executor: handle `null` for anti join properly #8706

eurekaka · 2018-12-17T08:20:28Z

What problem does this PR solve?

Fix #8642

What is changed and how it works?

enhance joiner to be aware of whether the result of join condition is false or null, and act correspondingly for AntiSemiJoin and LeftOuterAntiSemiJoin;
differentiate not exists from not in / != all, since they have various behaviors regarding null input;
for not in (subq), there are 2 challenges:
- executor of index join / hash join / merge join filters out null inner key before the real join operation;
- we don't differentiate in condition and equal conditions pulled up later in decorrelation now, but they should have various behaviors regarding null input as well;
so if any column of not in condition is possible to be null, we cannot apply decorrelate rule for it, otherwise we would generate wrong results.

Check List

Tests

Unit test
Integration test: this would break test cases in mysql_test, whose result is wrong previously. The result is updated in https://github.com/pingcap/tidb-test/pull/702

Code changes

Has interface methods change

Side effects

Possible performance regression: this may make some not in queries unable to decorrelate now, but correctness should be much more important than performance.

Related changes

Need to cherry-pick to the release branch

This change is

eurekaka · 2018-12-17T08:26:09Z

/run-all-tests

eurekaka · 2018-12-17T10:25:17Z

/run-all-tests tidb-test=pr/702

eurekaka · 2018-12-17T10:34:01Z

@zz-jason @winoros @lamxTyler PTAL

planner/core/logical_plan_builder.go

eurekaka · 2018-12-18T06:24:34Z

/run-all-tests tidb-test=pr/702

eurekaka · 2018-12-18T06:35:36Z

/run-unit-test

eurekaka · 2018-12-18T09:17:11Z

/run-all-tests tidb-test=pr/702

eurekaka · 2018-12-18T09:26:28Z

@winoros comments addressed, the failed unit-test is caused by goroutine leak in ddl package.

winoros

lgtm

planner/core/logical_plans.go

zhouqiang-cl · 2018-12-18T16:28:55Z

/run-unit-test

executor/joiner.go

expression/expression.go

eurekaka · 2018-12-20T02:33:36Z

@zz-jason comments addressed, PTAL

expression/expression.go

- enhance joiner to be aware of whether the result of join condition is false or null, and act correspondingly for AntiSemiJoin and LeftOuterAntiSemiJoin; - differentiate `not exists` from `not in / != all`, since they have various behaviors regarding null input; - for `not in (subq)`, there are 2 challenges: * executor of index join / hash join / merge join filters out null inner key before the real join operation; * we don't differentiate `in` condition and equal conditions pulled up later in decorrelation now, but they should have various behaviors either regarding null input; so if the inner column of `not in` condition is possible to be null, we cannot apply decorrelate rule for it, otherwise we would generate wrong results.

- rename isnull to hasNull - add comments

zz-jason · 2018-12-26T11:21:53Z

After some investigation from the behavior of MySQL, the join result of LeftOuterSemiJoin and Anti LeftOuterSemiJoin should be one of the following according to different conditions:

OuterRow + NULL
OuterRow + 1
OuterRow + 0

Here the OuterRow is a record from the left hand side table.

1. LeftOuterSemiJoin

If the outer join key contains NULL, the join result should be OuterRow + NULL, else:
If there is at least one inner row has the same join key with the outer row, the join result should be OuterRow + 1, else:
If there is at least one NULL in the inner join keys, the join result should be OuterRow + NULL, else:
the join result should be OuterRow + 0

2. Anti LeftOuterSemiJoin

If the outer join key contains NULL, the join result should be OuterRow + NULL, else:
If there is at least one inner row has the same join key with the outer row, the join result should be OuterRow + 0, else:
If there is at least one NULL in the inner join keys, the join result should be OuterRow + NULL, else:
the join result should be OuterRow + 1

@eurekaka Am I right?

eurekaka · 2018-12-27T05:37:50Z

@zz-jason yes, you are right.

This is the output of MySQL:

mysql> select * from s;
+------+------+
| a    | b    |
+------+------+
|    1 | NULL |
|    2 |    1 |
+------+------+
2 rows in set (0.00 sec)

mysql> select *, a in (select b from s) from s;
+------+------+------------------------+
| a    | b    | a in (select b from s) |
+------+------+------------------------+
|    1 | NULL |                      1 |
|    2 |    1 |                   NULL |
+------+------+------------------------+
2 rows in set (0.00 sec)

mysql> select *, a not in (select b from s) from s;
+------+------+----------------------------+
| a    | b    | a not in (select b from s) |
+------+------+----------------------------+
|    1 | NULL |                          0 |
|    2 |    1 |                       NULL |
+------+------+----------------------------+

while this is the output after applying this PR:

mysql> select * from t;
+------+------+
| a    | b    |
+------+------+
|    1 | NULL |
|    2 |    1 |
+------+------+
2 rows in set (0.00 sec)

mysql> select *, a in (select b from t) from t;
+------+------+------------------------+
| a    | b    | a in (select b from t) |
+------+------+------------------------+
|    1 | NULL |                      1 |
|    2 |    1 |                      0 |
+------+------+------------------------+
2 rows in set (0.00 sec)

mysql> select *, a not in (select b from t) from t;
+------+------+----------------------------+
| a    | b    | a not in (select b from t) |
+------+------+----------------------------+
|    1 | NULL |                          0 |
|    2 |    1 |                          0 |
+------+------+----------------------------+

For LeftOuterAntiSemiJoin, it is easy to update the code to generate compatible result with MySQL now, but for LeftOuterSemiJoin, we have to disable de-correlation for LogicalApply with LeftOuterSemiJoin first, because for queries like select *, a in (select a from t t2 where t2.b = t1.b) from t t1, if we de-correlate it to a normal join, t1.a = t2.a and t1.b = t2.b cannot be differentiated to behave differently regarding null input.

eurekaka · 2018-12-28T07:50:16Z

@zz-jason AntiLeftOuterSemiJoin works now, PTAL. LeftOuterSemiJoin would be handled later in a separate PR.

mysql> select * from s;
+------+------+
| a    | b    |
+------+------+
|    1 | NULL |
|    2 |    1 |
+------+------+
2 rows in set (0.00 sec)

mysql> select *, a not in (select b from s) from s;
+------+------+----------------------------+
| a    | b    | a not in (select b from s) |
+------+------+----------------------------+
|    1 | NULL |                          0 |
|    2 |    1 |                       NULL |
+------+------+----------------------------+

eurekaka · 2018-12-28T07:59:05Z

/rebuild

zz-jason · 2019-01-07T05:37:46Z

executor/joiner.go

 	}

 	for inner := inners.Current(); inner != inners.End(); inner = inners.Next() {
 		j.makeShallowJoinRow(j.outerIsRight, inner, outer)

-		matched, err = expression.EvalBool(j.ctx, j.conditions, j.shallowRow.ToRow())
+		matched, _, err = expression.EvalBool(j.ctx, j.conditions, j.shallowRow.ToRow())


Should we also maintain the correctness of hasNull for semi join? If not, how about adding some comment about why we don't care whether there is any NULL value in the inner side and make the return value hasNull to false?

For semi join, false and null are same because in both cases the outer row would not be outputted. I will add some comments.

zz-jason · 2019-01-07T07:53:41Z

executor/joiner.go

 	}

 	for inner := inners.Current(); inner != inners.End(); inner = inners.Next() {
 		j.makeShallowJoinRow(false, inner, outer)

-		matched, err = expression.EvalBool(j.ctx, j.conditions, j.shallowRow.ToRow())
+		matched, _, err = expression.EvalBool(j.ctx, j.conditions, j.shallowRow.ToRow())


This should be done after we differentiate whether the join key is from in or not, for example, here is the output from MySQL:

mysql> select a in (select b from t) from t; +------------------------+ | a in (select b from t) | +------------------------+ | 1 | | NULL | | NULL | | NULL | +------------------------+ 4 rows in set (0.00 sec) mysql> select exists(select * from t t2 where t2.b = t1.a) from t t1; +----------------------------------------------+ | exists(select * from t t2 where t2.b = t1.a) | +----------------------------------------------+ | 1 | | 0 | | 0 | | 0 | +----------------------------------------------+ 4 rows in set (0.00 sec) mysql> select a in (select a from t t2 where t2.b = t1.a) from t t1; +---------------------------------------------+ | a in (select a from t t2 where t2.b = t1.a) | +---------------------------------------------+ | 1 | | 0 | | 0 | | 0 | +---------------------------------------------+ 4 rows in set (0.00 sec)

zz-jason · 2019-01-07T07:55:53Z

executor/joiner.go

 }

 func (j *leftOuterSemiJoiner) onMatch(outer chunk.Row, chk *chunk.Chunk) {
 	chk.AppendPartialRow(0, outer)
 	chk.AppendInt64(outer.Len(), 1)
 }

-func (j *leftOuterSemiJoiner) onMissMatch(outer chunk.Row, chk *chunk.Chunk) {
+func (j *leftOuterSemiJoiner) onMissMatch(hasNull bool, outer chunk.Row, chk *chunk.Chunk) {


After maintaining the correct hasNull value, if hasNull is true, we should append a NULL value to make the correct join result.

eurekaka · 2019-02-25T08:26:32Z

The issue is fixed by another new PR: #9051, so we can close this now.

eurekaka added type/bugfix This PR fixes a bug. sig/planner SIG: Planner sig/execution SIG execution labels Dec 17, 2018

eurekaka changed the title ~~plan/executor: handle null for AntiSemiJoin properly~~ plan/executor: handle null for anti join properly Dec 17, 2018

eurekaka added the status/all tests passed label Dec 17, 2018

winoros reviewed Dec 18, 2018

View reviewed changes

planner/core/logical_plan_builder.go Outdated Show resolved Hide resolved

winoros reviewed Dec 18, 2018

View reviewed changes

planner/core/logical_plans.go Show resolved Hide resolved

eurekaka added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 18, 2018

zz-jason reviewed Dec 19, 2018

View reviewed changes

executor/joiner.go Outdated Show resolved Hide resolved

executor/joiner.go Show resolved Hide resolved

expression/expression.go Show resolved Hide resolved

expression/expression.go Outdated Show resolved Hide resolved

alivxxx reviewed Dec 21, 2018

View reviewed changes

expression/expression.go Show resolved Hide resolved

eurekaka added 5 commits December 25, 2018 08:34

left key cannot be null either

ef060e7

address comments: add flagDecorrelate and mark LogicalApply

d722e5b

add comments for cannotDecorrelate

4abb4cb

address comments:

a5c4f95

- rename isnull to hasNull - add comments

eurekaka force-pushed the anti-join-apply branch from 30a2728 to a5c4f95 Compare December 26, 2018 06:57

zz-jason closed this Dec 26, 2018

zz-jason reopened this Dec 26, 2018

Merge branch 'master' into anti-join-apply

f114e24

zz-jason mentioned this pull request Dec 27, 2018

*: derive col is not null from col op col conditions #8603

Merged

eurekaka added 2 commits December 28, 2018 02:11

return null instead of 0 for AntiLeftOuterJoin

c3b091b

Merge branch 'master' into anti-join-apply

fa21a81

Merge branch 'master' into anti-join-apply

211e402

eurekaka mentioned this pull request Dec 28, 2018

Semi Join should be NULL-Aware #8844

Closed

zz-jason self-requested a review January 6, 2019 07:31

zz-jason reviewed Jan 7, 2019

View reviewed changes

eurekaka closed this Feb 25, 2019

eurekaka deleted the anti-join-apply branch February 25, 2019 08:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

plan/executor: handle `null` for anti join properly #8706

plan/executor: handle `null` for anti join properly #8706

eurekaka commented Dec 17, 2018 •

edited

Loading

eurekaka commented Dec 17, 2018

eurekaka commented Dec 17, 2018

eurekaka commented Dec 17, 2018

eurekaka commented Dec 18, 2018

eurekaka commented Dec 18, 2018

eurekaka commented Dec 18, 2018

eurekaka commented Dec 18, 2018

winoros left a comment

zhouqiang-cl commented Dec 18, 2018

eurekaka commented Dec 20, 2018

zz-jason commented Dec 26, 2018 •

edited

Loading

eurekaka commented Dec 27, 2018

eurekaka commented Dec 28, 2018 •

edited

Loading

eurekaka commented Dec 28, 2018

zz-jason Jan 7, 2019

eurekaka Jan 7, 2019

zz-jason Jan 7, 2019

eurekaka Jan 7, 2019

zz-jason Jan 7, 2019

eurekaka Jan 7, 2019

eurekaka commented Feb 25, 2019

plan/executor: handle null for anti join properly #8706

plan/executor: handle null for anti join properly #8706

Conversation

eurekaka commented Dec 17, 2018 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

eurekaka commented Dec 17, 2018

eurekaka commented Dec 17, 2018

eurekaka commented Dec 17, 2018

eurekaka commented Dec 18, 2018

eurekaka commented Dec 18, 2018

eurekaka commented Dec 18, 2018

eurekaka commented Dec 18, 2018

winoros left a comment

Choose a reason for hiding this comment

zhouqiang-cl commented Dec 18, 2018

eurekaka commented Dec 20, 2018

zz-jason commented Dec 26, 2018 • edited Loading

1. LeftOuterSemiJoin

2. Anti LeftOuterSemiJoin

eurekaka commented Dec 27, 2018

eurekaka commented Dec 28, 2018 • edited Loading

eurekaka commented Dec 28, 2018

zz-jason Jan 7, 2019

Choose a reason for hiding this comment

eurekaka Jan 7, 2019

Choose a reason for hiding this comment

zz-jason Jan 7, 2019

Choose a reason for hiding this comment

eurekaka Jan 7, 2019

Choose a reason for hiding this comment

zz-jason Jan 7, 2019

Choose a reason for hiding this comment

eurekaka Jan 7, 2019

Choose a reason for hiding this comment

eurekaka commented Feb 25, 2019

plan/executor: handle `null` for anti join properly #8706

plan/executor: handle `null` for anti join properly #8706

eurekaka commented Dec 17, 2018 •

edited

Loading

zz-jason commented Dec 26, 2018 •

edited

Loading

eurekaka commented Dec 28, 2018 •

edited

Loading